Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add num_floors_underground #217

Merged
merged 5 commits into from
Jul 3, 2024
Merged

Add num_floors_underground #217

merged 5 commits into from
Jul 3, 2024

Conversation

skmoore
Copy link
Contributor

@skmoore skmoore commented Jun 12, 2024

Description

Adds a num_floors_underground flag for buildings that have one or more floors below ground level. This will allow for more accurate 3d representations of buildings that are partially or fully below ground. This should map to the building:levels:underground tag from OSM.

Reference

List of relevant links to GitHub issues, PRs, and other documentation.

  1. TODO.

  2. Add relevant examples.

  3. Add relevant counterexamples.

  4. Update any counterexamples that became obsolete. For example, if a counterexample uses property A but is not intended to test property A's validity, and you made a schema change that invalidates property A in that counterexample, fix the counterexample to align it with your schema change.

  5. Update in-schema documentation using plain English written in complete sentences, if an update is required.

  6. Update Docusaurus documentation, if an update is required.

  7. Review change with Overture technical writer to ensure any advanced documentation needs will be taken care of, unless the change is trivial and would not affect the documentation.

Documentation Website

Update the hyperlink below to put the pull request number in.

[Docs preview for this PR.](https://dfhx9f55j8eg5.cloudfront.net/pr/<PUT THE PR # HERE>)

@jwass jwass changed the base branch from main to dev June 13, 2024 02:17
@jwass jwass changed the title Add numfloors_underground Add num_floors_underground Jun 13, 2024
jwass
jwass previously approved these changes Jun 14, 2024
Copy link
Collaborator

@vcschapp vcschapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skmoore Would you mind adding one counterexample that has an invalid value for num_floors_underground and uses the ext_expected_exception property to verify that the right validation exception is generated?

@jenningsanderson jenningsanderson dismissed stale reviews from jwass and themself via c2fd79c June 17, 2024 21:44
jwass
jwass previously approved these changes Jun 18, 2024
Copy link
Collaborator

@jwass jwass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

DavidKarlas
DavidKarlas previously approved these changes Jun 18, 2024
@jenningsanderson
Copy link
Collaborator

@skmoore Would you mind adding one counterexample that has an invalid value for num_floors_underground and uses the ext_expected_exception property to verify that the right validation exception is generated?

I added a counter example, but I cannot generate the proper expected exception?

@vcschapp
Copy link
Collaborator

vcschapp commented Jun 26, 2024

@skmoore Would you mind adding one counterexample that has an invalid value for num_floors_underground and uses the ext_expected_exception property to verify that the right validation exception is generated?

I added a counter example, but I cannot generate the proper expected exception?

Can you help me understand what actions you did, and how the actual result different from your expectation?

The way I usually do this is:

  1. Add ext_expected_exceptions: ["some nonsense"]
  2. Run ./test.sh -m counterexamples "path/to/your/counterexample.yaml.
    • The test run should fail and print detailed output.
    • The reason it fails is because none of the failure outputs match the text "some nonsense".
  3. Look at the output.
    • Make sure there aren't any unexpected failures... If there are, the counterexample has another issue that needs fixing.
    • Find the expected failure line in the output.
  4. Replace "some nonsense" with a useful substring from the expected failure line.
  5. Repeat step 2, which should pass now.

@vcschapp vcschapp merged commit 05dc074 into dev Jul 3, 2024
2 checks passed
@vcschapp vcschapp deleted the numfloors_underground branch July 3, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants